Skip to content

Conversation

@brendt
Copy link
Member

@brendt brendt commented Apr 7, 2025

Alternative for #1105

@brendt brendt marked this pull request as draft April 7, 2025 07:51
@innocenzi
Copy link
Member

innocenzi commented Apr 25, 2025

I'm okay with moving forward with using dynamic initializers instead of the attributes. I agree that it's simpler after all.

EDIT: so I took a look at the changes and why the CI fails—it's because, as I thought, simply updating the database initializer to avoid the need for a chain of dependency (by instantiating manually the Connection) is not flexible enough. We use a different Connection in tests, so we can't just instantiate PDOConnection from within DatabaseInitializer. We need to resolve it from the container (by its interface), but then the right DatabaseConfig will not be used in the underlying classes.

@innocenzi innocenzi changed the title feat(container): add HasTag interface feat(container): support dynamic tags using dynamic initializers Apr 25, 2025
@brendt brendt added this to the v1.0.0-beta.1 milestone Apr 26, 2025
@brendt
Copy link
Member Author

brendt commented Apr 28, 2025

That's easily fixed: one initializer can register multiple singletons. It can do whatever it wants with the container. The #[Singleton] attribute is only there for short-hand convenience.

@brendt brendt marked this pull request as ready for review April 28, 2025 11:44
@innocenzi
Copy link
Member

Well, if you manage to fix it using this approach, it's awesome. I'll document the storage functionality after we merge this 👌

@coveralls
Copy link

Pull Request Test Coverage Report for Build 14724150077

Details

  • 23 of 23 (100.0%) changed or added relevant lines in 7 files are covered.
  • 23 unchanged lines in 5 files lost coverage.
  • Overall coverage decreased (-0.1%) to 80.147%

Files with Coverage Reduction New Missed Lines %
src/Tempest/Database/src/Config/MysqlConfig.php 1 0.0%
src/Tempest/Database/src/Config/PostgresConfig.php 1 0.0%
src/Tempest/Database/src/Connection/ConnectionInitializer.php 5 0.0%
src/Tempest/Database/src/Connection/CachedConnectionInitializer.php 7 0.0%
src/Tempest/Framework/Testing/IntegrationTest.php 9 75.68%
Totals Coverage Status
Change from base Build 14721086338: -0.1%
Covered Lines: 11849
Relevant Lines: 14784

💛 - Coveralls

@brendt brendt merged commit 0980e3a into main Apr 29, 2025
68 checks passed
@brendt brendt deleted the has-tags-interface branch April 29, 2025 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants